Skip to content

Initial Commit#2

Merged
eldadfux merged 199 commits intomainfrom
v0
May 13, 2021
Merged

Initial Commit#2
eldadfux merged 199 commits intomainfrom
v0

Conversation

@eldadfux
Copy link
Copy Markdown
Member

No description provided.

Comment thread tests/Database/Base.php
@eldadfux eldadfux requested a review from lohanidamodar May 12, 2021 11:03
Copy link
Copy Markdown
Contributor

@lohanidamodar lohanidamodar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey just few questions.

Comment thread src/Database/Database.php
Comment thread src/Database/Database.php
Comment thread src/Database/Adapter.php
Comment thread src/Database/Adapter/MariaDB.php
Comment thread src/Database/Adapter/MariaDB.php
Comment thread src/Database/Adapter/MariaDB.php
Comment thread src/Database/Adapter/MariaDB.php
Comment thread src/Database/Database.php
@eldadfux eldadfux merged commit a38ca8f into main May 13, 2021
eldadfux added a commit that referenced this pull request Jun 29, 2021
fogelito added a commit that referenced this pull request Apr 10, 2023
@coderabbitai coderabbitai Bot mentioned this pull request Jul 27, 2025
@coderabbitai coderabbitai Bot mentioned this pull request Aug 20, 2025
@coderabbitai coderabbitai Bot mentioned this pull request Nov 23, 2025
premtsd-code added a commit that referenced this pull request Apr 13, 2026
#1 Drop $enable flag on skipDuplicates() scope guard
The $enable param made every non-skipDuplicates createDocuments call pay
for a closure allocation + extra function call. Branch at the call site
instead so the cost only applies when the flag is actually set.

  - Adapter::skipDuplicates(callable, bool) → skipDuplicates(callable)
  - Database::skipDuplicates(callable, bool) → skipDuplicates(callable)
  - Database::createDocuments, Mirror::createDocuments, Pool::delegate,
    Pool::withTransaction now branch inline.

#2 Drop fetchExistingByIds helper, inline find()
The helper's per-tenant grouping defended a hypothetical multi-tenant
batching scenario that no caller exercises (relationships are intra-
tenant, callers always batch per tenant). Existing patterns in the same
file (refetchDocuments, relationship loading) just call find() directly.
Match that idiom and drop ~70 lines.

#4 Mirror: only capture inserted docs in skipDuplicates mode
The captureOnNext accumulator paid the cost (closure + per-doc array
push) on every createDocuments call, including the common non-skip path.
Branch at the entry of Mirror::createDocuments so the capture only
happens when skipDuplicates is set; the non-skip path passes through
to source/destination unchanged.

#5 Move getInsertKeyword/Suffix/PermissionsSuffix to getters cluster
Were sitting next to createDocuments(); moved to the getSupport*
cluster around line 1030 where other adapter-capability shims live.

Not addressed:
- #2 partial: the existing patterns (refetchDocuments etc.) don't handle
  tenant-per-document multi-tenant batches either, so this is consistent.
- #3 (drop the pre-filter): rejected. createDocumentRelationships runs
  in the encoding loop BEFORE the adapter's INSERT IGNORE no-ops the
  parent, so dropping the pre-filter would deterministically duplicate
  child rows on every CSV re-import of a collection with relationships
  (not a race window — every call). The relationships test verifies
  this. Reverting would require reintroducing the deferred-relationships
  scaffolding we just removed, and the adapter still couldn't tell us
  which parents were actually inserted (SQL INSERT IGNORE has no per-row
  reporting). Pre-filter stays.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants